Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add entities attr_reader to Twitter::Status #245

Merged
merged 5 commits into from
Mar 27, 2012

Conversation

tomykaira
Copy link
Contributor

Twitter::Status now provides limited access to fields of entities.

I often need full access to urls, and others may want to access other fields.

I prefer just adding a reader to writing methods for each set of fields.

@sferik
Copy link
Owner

sferik commented Mar 27, 2012

I'd prefer to see a nicer interface for accessing Tweet Entities than just returning the hash from Twitter API.

I'm imagining instance methods named hashtags, urls, and user_mentions defined on the Twitter::Status object. These methods should emit a warning when they are called but the entities hash does not exist (telling the user they need to pass :include_entities => true when they fetch the Twitter::Status object).

Would you like to implement this or should I?

@tomykaira
Copy link
Contributor Author

Thanks @sferik, that is great idea!

I implemented, but I am not sure how to warn an user about :include_entities.

Twitter::Error seems to be for Twitter API errors, and this error only occurs here. So I packed this error under the Status class. But it is not so cool.

Please check this patch and let me know more about your image.

Thanks, in advance.

@sferik
Copy link
Owner

sferik commented Mar 27, 2012

This isn't perfect, but I'm going to merge it and then clean it up before releasing a new version.

sferik added a commit that referenced this pull request Mar 27, 2012
Add entities attr_reader to Twitter::Status
@sferik sferik merged commit 32375d9 into sferik:master Mar 27, 2012
sferik added a commit that referenced this pull request Mar 27, 2012
@tomykaira
Copy link
Contributor Author

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants